-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: fix flakiness of stringbytes-external #6039
Conversation
9f7fe64
to
7ae1e9c
Compare
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945
7ae1e9c
to
4e6f03c
Compare
Interesting. I believe all the other tests in |
Indeed, although my interpretation of An uglier alternative would be to expose the |
This is certainly an interesting case. Overall it LGTM it's just definitely quite a bit different from our other tests. |
LGTM |
Would perhaps the cctest suite be a better fit? |
Adding a new suite for this is probably overkill. I don't think this pattern is going to be very common to require a suite. I would like to land this PR today as it is blocking #5945 which needs to land very soon in order to get ready for the Node v6 RC schedule. |
LGTM. After it lands, we can update the README in the test directory to call out this particular test. |
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945 PR-URL: nodejs#6039 Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: evanlucas - Evan Lucas <evanlucas@me.com> Reviewed-By: Trott - Rich Trott <rtrott@gmail.com>
Thanks. Landed as f4ebd59. I will update the README in a follow-on. |
Closing, as the changes have landed already. |
Follow-on: #6073. |
Avoid depending on precise timing of when an object will be collected by GC. This test was missed by nodejs#6039 as it happened to be in a different directory than the rest. Ref: nodejs#6039 PR-URL: nodejs#6073 Reviewed-By: Trott - Rich Trott <rtrott@gmail.com>
@ofrobots this is not merging cleanly onto v5.x, would you be able to backport? |
since this is not landing on v5.x I'm marking as We will still want this backported to |
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945 Ref: nodejs#6039 Ref: nodejs#6073
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
Pull Request check-list
make -j8 test
(UNIX) orvcbuild test nosign
(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
Affected core subsystem(s)
test
Description of change
The tests used to rely on precise timing of when a JavaScript object
would be garbage collected to ensure that there is enough memory
available on the system. Switch the test to use a malloc/free pair
instead.
Ref: #5945
R=@bnoordhuis, @Trott
CI: https://ci.nodejs.org/job/node-test-pull-request/2146/CI: https://ci.nodejs.org/job/node-test-pull-request/2147/CI: https://ci.nodejs.org/job/node-test-pull-request/2148/